Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

In blockstore tests treat Close and Closing the same way #2832

Closed
wants to merge 1 commit into from

Conversation

Kubuxu
Copy link
Member

@Kubuxu Kubuxu commented Jun 9, 2016

This should prevent race condition that caused #2762

Resolves #2762

License: MIT
Signed-off-by: Jakub Sztandera [email protected]

@whyrusleeping
Copy link
Member

I'm not sure i follow how this solves the issue, could you elaborate?

@Kubuxu
Copy link
Member Author

Kubuxu commented Jun 9, 2016

If the cancel is run in the same goroutine then it will send signal to the already waiting goroutine which will cause the goprocess to close. By running it in separate goroutine we should give ourselves enough time to reach the select statement before the cancel causes the goprocess to close.

@Kubuxu
Copy link
Member Author

Kubuxu commented Jun 10, 2016

This fix doesn't work as I thought it would.
As I am able to replicate the issue I might be able to fix it now.

As there is no piority system for select and it chooses channel
at random if multiple are read and time difference between Close and
Closing is very small it is best solution to this problem.

License: MIT
Signed-off-by: Jakub Sztandera <[email protected]>
@Kubuxu Kubuxu force-pushed the feature/test-datastore branch from 87cd2ea to 522c590 Compare June 10, 2016 13:42
@Kubuxu Kubuxu changed the title In blockstore tests run cancel in separate goroutine In blockstore tests treat Close and Closing the same way Jun 10, 2016
@Kubuxu
Copy link
Member Author

Kubuxu commented Jun 10, 2016

I've changed the approach as that one was not working and I was able to replicate the issue by repeatability running tests.

@Kubuxu
Copy link
Member Author

Kubuxu commented Jun 10, 2016

Ok, it looks like it isn't test inconsistency after all. I am getting failures at https://github.com/ipfs/go-ipfs/pull/2832/files#diff-89a41509a8389c8bc3dbf7594aa13416R192 now.

@Kubuxu Kubuxu closed this Jun 10, 2016
@whyrusleeping whyrusleeping deleted the feature/test-datastore branch June 10, 2016 15:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants